Correctly handle wares carried into harbors with no path to goal #1852
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
it seems as if I drove the logistics into an interesting corner case? :-) |
Flow86
requested changes
Jan 5, 2026
The check `GetGoal() != GetLocation()` is always met because the location is the flag and the goal is NULL if it was destroyed while the ware is being carried out.
When the goal for a ware becomes unreachable while it is carried into a harbor `RecalcRoute` will call `FindRouteToWarehouse`. Usually the selected warehouse will be the harbor as it is trivially the closest. However when the harbor does not accept this ware type another one might be chosen and the ware needs to be moved out instead of stored. But `FindRouteToWarehouse` does NOT set `next_dir` while the ware is carried even when called from `RecalcRoute` where the caller assumes it is set. Changing this causes replays to go async, so work around it by calling `RecalcRoute` again which now succeeds as it reached the goal in the previous call. This bug also affects `WantInBuilding` which also calls `RecalcRoute` assuming it does so in all cases. Fixes Return-To-The-Roots#1843
Flow86
previously approved these changes
Jan 12, 2026
Member
Author
Yeah, it's a timing issue and hence difficult to reproduce in "normal" circumstances so I cheated a bit for the test case. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the goal for a ware becomes unreachable while it is carried into a harbor
RecalcRoutewill callFindRouteToWarehouse.Usually the selected warehouse will be the harbor as it is trivially the closest.
However when the harbor does not accept this ware type another one might be chosen
and the ware needs to be moved out instead of stored.
But
FindRouteToWarehousedoes NOT setnext_dirwhile the ware is carried even whencalled from
RecalcRoutewhere the caller assumes it is set.Changing this causes replays to go async, so work around it by calling
RecalcRouteagainwhich now succeeds as it reached the goal in the previous call.
This bug also affects
WantInBuildingwhich also callsRecalcRouteassuming it does so in all cases.Fixes #1843
The test case was a bit tricky to write and I added the case where no goal is possible in which case the ware should be accepted in the ware house anyway.